-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Groups #80
base: main
Are you sure you want to change the base?
Groups #80
Conversation
Very excited to read through and test this. However, from a quick glance it does not conform to our code standards (see CONTRIBUTING) and has a lot of unmotivated changes. I will code review it as soon as I can. Has this been tested with the NAT punching as well? And great job! Thanks. |
No worries I'll take a look at the standards a bit later and get it up to scratch. Haven't tested with NAT punching but will give it a shot. Server updates not far behind so you'll be able to play with it all |
Perfect, thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but some changes are required before merging and I also have some concerns that needs to be addressed.
Fika.Core/UI/Patches/MatchmakerAcceptScreen/MatchmakerAcceptScreen_Show_Patch.cs
Outdated
Show resolved
Hide resolved
5bea97c
to
39eb0ef
Compare
Alrighty those changes have been made |
@Lacyway anything holding this back atm? |
It's severely outdated and I still haven't figured out how to bypass the hardcoded max players. I've asked the author to keep it so that we can try to implement it later. |
Adds: